Skip to content

Conversation

taeold
Copy link
Contributor

@taeold taeold commented Aug 17, 2025

Summary

Adds a converter to transform Firebase Extensions (extension.yaml) into Functions Build format, laying groundwork for future extension deployment support.

Key Changes

  • Added extensionAdapter.ts to convert extension.yaml to Functions Build format
  • Converts extension parameter syntax ${param:NAME} to CEL expressions {{ params.NAME }} for later resolution
  • Supports all trigger types (HTTPS, Event, Schedule, Task Queue) for both v1 and v2 functions
  • Leverages existing utilities from specHelper and proto for clean implementation (~330 lines)

Test Plan

  • All existing tests pass
  • Added comprehensive test suite with 8 golden test fixtures
  • Validates conversion of real extension.yaml patterns

Note

This PR adds the conversion capability only. Integration with the deploy command will come in a future PR.

taeold added 10 commits August 16, 2025 21:19
### Description
Implements an adapter that converts Firebase Extensions (extension.yaml) to Functions Build format,
enabling deployment of extensions using `firebase deploy --only functions`.

### Key Changes
- Add extensionAdapter module to convert extension.yaml to Build objects
- Support for all extension resource types (v1 and v2 functions)
- Parameter reference conversion from ${param:X} to CEL {{ params.X }}
- Comprehensive golden tests with 7 test fixtures
- Fix typo in types: maxConcurrentDispatchs → maxConcurrentDispatches
- Add resourceType field to Param interface for SELECT_RESOURCE params

### Scenarios Tested
- Simple HTTPS functions
- Firestore triggers with parameter references
- Scheduled functions
- Task queue functions with rate limits
- v2 functions with event filters
- Storage resize extension with complex parameters
- Various parameter types (string, select, multiSelect, secret)

### Sample Commands
```bash
# Run tests
npx mocha lib/deploy/functions/extensionAdapter.golden.spec.js

# Future usage (after integration):
firebase deploy --only functions  # Will auto-detect extension.yaml
```

This is the foundation for supporting extension.yaml files in Functions deployment.
Next steps include integrating into Node.js runtime discovery.
- Rename extensionAdapter.golden.spec.ts to extensionAdapter.spec.ts
- Remove unnecessary comments
- Remove error silencing in getFixtures() for explicit test failures
- Remove unnecessary type guard for parameter references
- Use existing k8s.mebibytes() utility instead of duplicating memory parsing
- Remove duplicate parseMemory() function
- Simplify parameter reference checks to inline conditions
- Run formatter to clean up code style
- Extract trigger building into separate functions (buildV1Trigger, buildV2Trigger)
- Create unified createEndpoint function that handles both v1 and v2
- Remove all 'any' types, use proper type narrowing instead
- Simplify convertResources to use single endpoint creation path
- Reduce code duplication while maintaining type safety
- Extract helper functions: buildOptions, buildTextInput, convertParam
- Replace nested if-else chain with cleaner switch statement
- Use functional approach with map instead of for loop
- Separate concerns into smaller, focused functions
- Make code more maintainable and easier to extend
- Extensions already specify runtime per resource in the YAML
- Remove unnecessary runtime parameter from all function signatures
- Use runtime directly from extension resource properties
- Add validation to ensure runtime is specified for each function
- Remove Build-level runtime since each endpoint has its own
- Update tests and expected files accordingly
- Remove unused sourceDir parameter from detectAndAdaptExtension
- Add deployment-specific validation (name, version, resources required)
- Extract magic string to DEFAULT_RESOURCE_TYPE constant
- Add comprehensive JSDoc for public API
- Update tests to match new function signature

Note: Using custom validation instead of validateSpec() since that's
designed for publishing extensions (requires license, etc.) while we
only need basic validation for deployment.
- Remove verbose displayIAMRoles function
- Replace with simple log message for IAM roles
- Add clear TODO comments for future IAM integration
- Add TODO for lifecycle events support
- Remove unused Role import

These features aren't critical for initial implementation and can be
added when needed for full extension compatibility.
Reduced extensionAdapter.ts from 534 to 346 lines (35% reduction) by:
- Leveraging readExtensionYaml from specHelper for parsing
- Using getResourceRuntime and DEFAULT_RUNTIME from existing utils
- Removing duplicate runtime extraction logic
- Cleaning up trigger handling without hacky delete operations
- Fixing TypeScript types to avoid any casts
- Consolidating v1/v2 logic into single createEndpoint function

All tests pass and functionality remains identical.
- Use proto.copyIfPresent and proto.renameIfPresent instead of manual null checks
- Remove unnecessary inline comments
- Revise JSDoc comments to describe WHAT not WHY
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary of Changes

Hello @taeold, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a new converter that transforms Firebase Extensions defined in extension.yaml into the Functions Build format. This is a crucial step towards enabling the deployment of Firebase Extensions directly through the Firebase CLI, laying the groundwork for future integration with the deploy command.

Highlights

  • Core Conversion Logic: A new file extensionAdapter.ts is added, responsible for converting extension.yaml specifications into the Functions Build format.
  • Parameter Syntax Transformation: The converter handles the transformation of extension parameter syntax (e.g., ${param:NAME} or ${NAME}) into Common Expression Language (CEL) expressions ({{ params.NAME }}), which are used for later resolution.
  • Comprehensive Trigger Support: The adapter supports all major Cloud Functions trigger types, including HTTPS, Event, Schedule, and Task Queue triggers, for both v1 and v2 functions.
  • Code Reusability: The implementation leverages existing utility functions from specHelper and proto modules, ensuring a clean and efficient codebase.
  • Extensive Test Coverage: A new test file extensionAdapter.spec.ts is introduced, along with 8 golden test fixtures, to thoroughly validate the conversion logic across various extension.yaml patterns.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a converter to transform extension.yaml files into the Functions Build format, which is a great step towards supporting extension deployments. The implementation is comprehensive, covering various trigger types and parameter configurations for both v1 and v2 functions. The addition of a thorough golden test suite is excellent for ensuring correctness. My review includes a few suggestions to improve robustness and type safety in the new adapter logic.

…tests

- Fix parameter substitution checks to verify both opening and closing braces
- Add radix parameter to parseInt for memory parsing
- Replace 'any' type with proper TextInput type from params module
- Add support for parsing memory values in IEC notation (1GiB, 512MiB)
  - Extensions use IEC notation while k8s.mebibytes expects Kubernetes notation
  - Convert by stripping 'B' suffix (both represent same binary values)
- Add test coverage for 1GiB memory edge case
- Add test coverage for Firestore trigger with match-path-pattern operator
@taeold
Copy link
Contributor Author

taeold commented Aug 17, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a well-structured and thoroughly tested converter to transform extension.yaml files into the Functions Build format. The implementation is comprehensive, covering various function triggers and parameter types for both v1 and v2 functions. My review includes two main points: one to enhance robustness by adding validation for a required parameter property to prevent silent misconfigurations, and another to improve code clarity by simplifying some conditional logic and removing redundant code. Overall, this is a solid contribution that lays important groundwork for future extension deployment capabilities.

Major improvements:
- Refactored monster createEndpoint function into createV1Endpoint and createV2Endpoint
- Extracted parseMemoryToMb and parseTimeout helper functions for better reusability
- Fixed type issues: parseTimeout accepts string only, return types use Field<number>
- Better use of proto.* helpers (convertIfPresent, copyIfPresent, renameIfPresent)
- Removed DEFAULT_RUNTIME import from emulator, use supported.latest("nodejs") directly
- Improved type safety throughout - removed all type assertions
- Fixed lint warnings: removed unnecessary string assertions, proper error handling without any

The code is now more maintainable, follows established patterns in the codebase,
and has proper type safety without cheating the type system.
@taeold
Copy link
Contributor Author

taeold commented Aug 18, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a converter to transform extension.yaml files into the Functions Build format, which is a significant step towards supporting extension deployments. The implementation is comprehensive, covering various trigger types, parameter types, and both v1 and v2 functions. The addition of a thorough golden test suite is excellent for ensuring correctness. My review includes a few suggestions for refactoring to improve code consistency and maintainability, and to address a potential issue with a fallback value.

taeold added 2 commits August 17, 2025 19:48
- Use more robust regex for hasParamReference to avoid false positives
- Simplify v1 endpoint to use proto.convertIfPresent for all field conversions
- Simplify v2 endpoint min/max instances handling with proto.convertIfPresent
- Fix parseTimeout function to be actually used (was previously unused)
- Remove incorrect default for selectResource resourceType - it's a required field
  and should error if missing, not silently use a default

All tests passing and no lint warnings.
- Remove redundant file system check - directly try reading extension.yaml
  and handle ENOENT gracefully (one I/O operation instead of two)
- Remove path import as it's no longer needed
- Properly wrap errors with context when extension.yaml read fails
- Remove obvious/redundant comments throughout the code
- Keep only meaningful comments (IEC notation explanation, TODOs)
- Fix all lint and formatting issues

This completes all the code review feedback. The code is now cleaner,
more efficient, and follows best practices.
@taeold
Copy link
Contributor Author

taeold commented Aug 18, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a converter to transform extension.yaml files into the Functions Build format, which is a great step towards supporting extension deployments. The implementation is comprehensive, covering various trigger types for both v1 and v2 functions, and correctly handles parameter substitutions. The accompanying test suite with golden fixtures is also well-designed.

I've identified a couple of areas for improvement to enhance the robustness of the tests and the clarity of the implementation. My comments focus on making a test case less brittle and improving the precision of a condition for parameter processing.

taeold added 2 commits August 17, 2025 21:44
Add default database and namespace filters to Firestore v2 event triggers
when not explicitly provided, matching the emulator's behavior.
Update the expected output to include the namespace field that's now
automatically added for Firestore v2 triggers.
@taeold
Copy link
Contributor Author

taeold commented Aug 18, 2025

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a converter to transform Firebase Extensions (extension.yaml) into the Functions Build format, which is a crucial step towards supporting extension deployment. The implementation is well-designed, handling various function versions (v1/v2), trigger types, and parameter substitutions. The inclusion of a comprehensive suite of golden tests is particularly commendable as it ensures the correctness of this complex transformation logic. The code is clean and follows existing patterns well. I have one minor suggestion for improving code clarity.

taeold added 3 commits August 18, 2025 10:53
The build.empty() function already initializes endpoints to an empty
object, so the explicit assignment is unnecessary.
- Extension multiSelect params now properly map to list type instead of string type
- Default values are converted from comma-separated strings to arrays
- Updated test fixtures to reflect correct behavior
- Fixes 'Non-list params cannot have multi selector inputs' error
taeold added 2 commits August 19, 2025 10:41
- Added detection for select params with all numeric values (e.g., FUNCTION_MEMORY)
- These are converted to IntParam to allow proper type coercion in numeric contexts
- Fixes 'Illegal type coercion' error when using FUNCTION_MEMORY with availableMemoryMb
- Legacy pattern used by ~4% of extensions, later replaced by system params
- Added test case for numeric select param conversion
…n adapter

- Documented how system params work (auto-generated, advanced, not env vars)
- Listed common system params (location, memory, timeout, etc.)
- Explained differences between v1 and v2 functions (memory units)
- Noted that full system param support would require detecting function types and user configuration
- For local testing, we rely on properties defaults or platform defaults
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant